Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always evaluate the context file in a separate process #313

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JamesWrigley
Copy link
Member

The GUI used to have a special case for databases using the default environment where it would evaluate the context file in the same process. This is bad because then the context file might not be evaluated in the database directory, which would break local imports. Now we always evaluate it with get_context_file() in a separate process in the database directory.

(CC @turkot)

@JamesWrigley JamesWrigley added the bug Something isn't working label Aug 13, 2024
@JamesWrigley JamesWrigley self-assigned this Aug 13, 2024
@JamesWrigley
Copy link
Member Author

sigh the tests passed locally for me 🤦 I'll look into it tomorrow.

@takluyver
Copy link
Member

LGTM, thanks!

Do we want to also remove the context_python is None in get_context_file, so that always uses a subprocess as well? I think it's becoming normal to use a separate Python for the context file, so the extra code for a fast path is less useful than it was.

The GUI used to have a special case for databases using the default environment
where it would evaluate the context file in the same process. This is bad
because then the context file might not be evaluated in the database directory,
which would break local imports. Now we always evaluate it with
`get_context_file()` in a separate process in the database directory.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants